Skip to content

feat(adapters/slack): add slackapi, a low-level Slack API package - #97

Draft
ethanndickson wants to merge 24 commits into
mainfrom
ethan/slackapi
Draft

ethanndickson wants to merge 24 commits into
mainfrom
ethan/slackapi

Conversation

@ethanndickson

@ethanndickson ethanndickson commented Sep 30, 2026 •

Copy link
Copy Markdown
Member

The Slack adapter already does all the Slack protocol work, but you can only get at it through chat.New. An application that runs its own chat engine, with its own state, dispatch, and routing (say, a Slack bot built into another server), can't use it. It also needs things the portable surface leaves out on purpose, like edits, deletes, reactions, and files.

This PR adds adapters/slack/slackapi, a low-level package for that: a typed Web API client with bounded retry, the request signature check, Events API parsing, file upload and host-restricted download, Block Kit types, and a Markdown splitter for Slack's markdown block limit. There's also slackapitest, a fake Slack server for callers to test against. ADR 0016 has the reasoning.

The adapter now uses slackapi for its calls, retry, signature check, and history reads, so there's only one copy of each. RetryPolicy and RateLimited are now aliases, so nothing breaks. The one behaviour change is that a rate_limited error in a 200 response now retries too, the same as a 429. The adapter still builds its own request bodies for auth.test, chat.postMessage, chat.postEphemeral and conversations.open, and I'll move those onto the typed methods in a follow-up.

There's also an env-gated TestLive, which I ran against my test workspace, and it passed. It showed that the 12,000 character limit applies to all the markdown blocks in a message together, not to each block. It also showed that the adapter's HistoryReader was broken on real Slack, since it sent conversations.history and conversations.replies as JSON:

slack: conversations.replies failed: invalid_arguments (code "invalid_arguments", detail "[ERROR] missing required field: channel; [ERROR] missing required field: ts", HTTP status 200)

slackapi sends those as form posts, which works, so the adapter now just reads history through slackapi. The adapter tests' fake Slack server now rejects JSON for those methods too, like real Slack does.

Most of the diff is tests. The first few commits split it up by area, and the rest are fixes on top.

…rects and check upload redirects

DownloadFile sets the bearer token again on each redirect that passes the FileOrigins check, because http.Client drops it on a redirect to another host. UploadToURL checks the origin of every redirect with a per-call copy of the HTTP client and never adds a token. Origins drop the default port of the scheme, New warns about each dropped FileOrigins entry, and an empty file URL returns a clear error.
GetUploadURLExternal sends its form through callForm. The response type is now GetUploadURLExternalResponse, which matches CompleteUploadExternalResponse and does not stutter with its UploadURL field. UploadFile wraps step errors without a second slack: prefix.
… encode empty action elements

SplitMarkdown tracks the backtick count of the opening fence: only a line of at least as many backticks and optional whitespace closes it, and the added closing fence repeats that count. A chunk no longer ends inside or right after an opening fence line, which left an empty code block. ActionsBlock encodes nil Elements as an empty array.
@ethanndickson

Copy link
Copy Markdown
Member Author

@codex review

@ethanndickson

Copy link
Copy Markdown
Member Author

@codex security review

@chatgpt-codex-connector

chatgpt-codex-connector Bot commented Sep 30, 2026 •

Copy link
Copy Markdown

Codex Review Summary

This comment shows the latest Codex review activity on this pull request.

Review Status Commit Review trigger
📝 Code Review ✅ Completed 2026-10-05T13:39:06.030549Z 1e0e473 Manual request
🔒 Security Review ✅ Completed 2026-09-30T06:57:16.088072Z 1cc5463 Manual request
ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review" or "@codex security review".

Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings.

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 1cc5463fbd

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment thread adapters/slack/slackapi/chat.go
@ethanndickson

Copy link
Copy Markdown
Member Author

@codex review

@chatgpt-codex-connector

Copy link
Copy Markdown

🛡️ Codex Security Review

Security review completed. No security issues were found in this pull request.

Reviewed commit: 1cc5463fbd

View security finding report

Only the user who started this review can view the report in Codex.

ℹ️ About Codex security reviews in GitHub

This is an experimental Codex feature. Security reviews are triggered when:

  • You comment "@codex security review"
  • A regular code review gets triggered (for example, "@codex review" or when a PR is opened), and you’re opted in so security review runs alongside code review

Once complete, Codex will leave suggestions, or a comment if no findings are found.

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 1cc5463fbd

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment thread adapters/slack/slackapi/files.go
Comment thread adapters/slack/slackapi/files.go
@ethanndickson

Copy link
Copy Markdown
Member Author

@codex review

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: a4d7d5e183

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment thread adapters/slack/history.go Outdated
@ethanndickson

Copy link
Copy Markdown
Member Author

@codex review

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: e887367dee

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment thread adapters/slack/slackapi/blocks.go
@ethanndickson

Copy link
Copy Markdown
Member Author

@codex review

@chatgpt-codex-connector

Copy link
Copy Markdown

Codex Review: Didn't find any major issues. Breezy!

Reviewed commit: e887367dee

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

@ThomasK33

Copy link
Copy Markdown
Member

Maintainer review. I accept the direction. Below are a few targeted requests before this leaves draft.

Direction. Putting the Slack protocol code into a low-level slackapi package, below the supported adapter, fits the architecture. It sits outside the portable chat.* surface, as ADR 0004 intends, and the root module keeps zero external dependencies. ADR 0016 records decisions only, which is what we want. I'm not asking for a split: the adapter migration is what proves the client works against the existing hardening suites. Thank you also for running TestLive. It found a real production bug on main, filed as #98 with this PR as the proposed fix.

Requests:

  1. Tier label. Our charter says new surfaces start as experimental. Please mark slackapi and slackapitest as experimental and still evolving in their doc.go, for example "exported API may change before it is promoted". The existing Slack adapter keeps its supported tier. This only sets expectations for the new exports.
  2. Speculative exports. Keep what your downstream caller needs now. Could you justify the Manifest types and the assistant thread-status method, or make them unexported for now? We can export them later when someone needs them. Everything else is fine as it is.
  3. History regression evidence. This is the most important item. Please add tests that assert the actual form-encoded parameters sent to conversations.history and conversations.replies, including the cursor/pagination parameters. Please also add a test that newest-first order holds across cursor pages, not just within one reversed page. The bug(adapters/slack): HistoryReader fails on live Slack — conversations.history/replies sent as JSON #98 bug slipped through because the fake server accepted anything, so the tests should pin down the exact request format.
  4. Behavior changes in the PR body. Please list all three explicitly: history request encoding (fixes bug(adapters/slack): HistoryReader fails on live Slack — conversations.history/replies sent as JSON #98), reply ordering normalized to newest-first, and rate_limited retried when it comes back in an HTTP 200 response. Please add "Fixes bug(adapters/slack): HistoryReader fails on live Slack — conversations.history/replies sent as JSON #98".

After that, it's the normal gate: green CI, then one @codex review and one @codex security review on the final head (pushes reset both), and zero unresolved threads. The earlier clean verdicts don't carry over to later pushes. The typed-method follow-up for auth.test / chat.postMessage etc. sounds right. Please open a tracking issue for it when this merges.


Generated with mux • Model: anthropic:claude-fable-5 • Thinking: xhigh

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: e887367dee

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment thread adapters/slack/slackapi/doc.go
Comment thread adapters/slack/history_test.go Outdated
Comment thread adapters/slack/slackapi/manifest.go
Comment thread adapters/slack/slackapi/status.go
… and the app home manifest section

User gets IsRestricted and IsUltraRestricted, and Message gets UserTeam,
SourceTeam, Username, and BotProfile, so a bot can drop guests and users
from another organization and name the app behind a bot message.
ManifestFeatures gets AppHome, so a manifest can turn the Messages tab off.
@ethanndickson

Copy link
Copy Markdown
Member Author

@codex review

@chatgpt-codex-connector

Copy link
Copy Markdown

Codex Review: Didn't find any major issues. 🚀

Reviewed commit: 1e0e473af7

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants